Skip to content

fix(history): Remove duplicate history objects for entries#163

Merged
samoehlert merged 19 commits intomainfrom
topic/soehlert/duplicate_history_entries
Nov 3, 2025
Merged

fix(history): Remove duplicate history objects for entries#163
samoehlert merged 19 commits intomainfrom
topic/soehlert/duplicate_history_entries

Conversation

@samoehlert
Copy link
Copy Markdown
Collaborator

Closes #9
Closes #162

This removes the duplicate historicalentry we saw on initial creation. This also handles the comment on updates. This did require adding an update api view and serializer. The update stuff is only available on the API at the moment, we can add an update template for the WUI later if we want (likely).

As part of this change we fixed the messages getting swallowed. Sorry I can't git well enough to fix that and add to another branch, but it was such a minor code change hopefully it's ok.

Also, I did figure out that the changed_by field shown in the admin is actually the _historical_user field, however, I don't think that matters. It's only a cosmetic thing in the admin panel as the API correctly shows the who field. Since that's the field we care about (as it's calculated both from API calls and the webUI) we can just use that field in our front end templates.

Sam Oehlert and others added 6 commits July 7, 2025 17:37
…allows us to only have one historicalentry on initial entry.
…cted to home meant none of our messages showed up. this was because it loaded once quickly then again which is what the user would see
…but handle updating it on changes. also define some read only fields since users should not be allowed to change those things
@samoehlert samoehlert requested a review from crankynetman July 9, 2025 06:42
@samoehlert samoehlert self-assigned this Jul 9, 2025
@samoehlert samoehlert marked this pull request as draft July 9, 2025 06:42
@samoehlert samoehlert added bug Something isn't working ux User experience issues labels Jul 9, 2025
@crankynetman crankynetman changed the title fix(history): Remove duplicate entries fix(history): Remove duplicate history objects for entries Jul 9, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 9, 2025

File Coverage
All files 81%
config/consumers.py 78%
config/urls.py 69%
config/settings/base.py 69%
config/settings/local.py 72%
scram/route_manager/admin.py 71%
scram/route_manager/models.py 70%
scram/route_manager/views.py 86%
scram/route_manager/api/serializers.py 92%
scram/route_manager/api/views.py 74%
scram/shared/shared_code.py 56%
scram/templates/403.html 91%
scram/templates/404.html 91%
scram/templates/base.html 99%
scram/templates/route_manager/home.html 80%

Minimum allowed coverage is 50%

Generated by 🐒 cobertura-action against 998d8dd

…he route and actiontype in the view and passed it to the serializer
@crankynetman crankynetman marked this pull request as ready for review July 15, 2025 17:07
Copy link
Copy Markdown
Collaborator

@crankynetman crankynetman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few comments inline

Comment thread scram/route_manager/api/serializers.py
Comment thread scram/route_manager/api/views.py Outdated
Comment thread scram/route_manager/views.py
Comment thread scram/route_manager/api/views.py Outdated
soehlert and others added 7 commits October 21, 2025 10:15
* main:
  ci: remove test that doesn't work on GH Actions (#170)
  fix(typo): use the correct key
  feat(gw_priority): this should allow us to choose which network is the "default gateway" of the macvlan address
  style(isort): fix ruff isort failure
  refactor(update): use the newest mozilla oidc package
  fix(oidc): fix oidc auth. i was overly complex and also misunderstanding the settings
  fix(login/out/redirect): edit the log{in|out}_urls and log{in|out}_redirect_urls to fix the local auth and hopefully the oidc
  fix(compose): use proper user for postgres healthcheck (#159)
  fix: move to bookworm (#166)
…e not required

this also makes the scram_client experience better as it doesn't need to track entries. see https://github.com/esnet-security/SCRAM/pull/163/files#r2208200704
i think we do want to fail on the method, not on the user permissions. doing a 403 suggests that sometimes you can change an entry, but we always want to block PUT/PATCH as the client should just do a normal post every time and SCRAM handles the logic on our end. We don't want the client to have to track these things.
Copy link
Copy Markdown
Collaborator

@crankynetman crankynetman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. This was a tricky one to track down, nicely done!!

@samoehlert samoehlert merged commit 3044137 into main Nov 3, 2025
19 checks passed
@samoehlert samoehlert deleted the topic/soehlert/duplicate_history_entries branch November 3, 2025 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ux User experience issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

History is not grabbing the change reason for updates Duplicate history entry on Entry object

3 participants